src/main/java/project/flipnote/group/exception/GroupErrorCode.java (1)

17-17: 오탈자(띄어쓰기) 수정 제안

메시지의 자연스러운 띄어쓰기를 위해 아래처럼 수정하는 것을 제안드립니다.

- OTHER_USER_EXIST_IN_GROUP(HttpStatus.CONFLICT, "GROUP_005", "그룹내 오너 제외 유저가 존재합니다.");
+ OTHER_USER_EXIST_IN_GROUP(HttpStatus.CONFLICT, "GROUP_005", "그룹 내 오너 제외 유저가 존재합니다.");

src/main/java/project/flipnote/group/repository/GroupMemberRepository.java (1)

23-24: 존재 여부는 count 대신 exists 파생 쿼리 사용 권장 + 네이밍 일관성

현재 사용 목적이 "다른 멤버가 존재하는지만" 확인이라면 count보다 exists 파생 쿼리가 의도에 더 맞고 일반적으로 더 가볍습니다. 또한 같은 인터페이스 안에서 findByGroup_IdAndUser_Id와 케이스가 섞여 있으므로, 새 메서드는 CamelCase 경로 표기(Group_Id, User_Id)로 맞추는 것을 권장합니다.

제안 diff:

- long countByGroup_idAndUser_idNot(Long groupId, Long userId);
+ boolean existsByGroup_IdAndUser_IdNot(Long groupId, Long userId);

서비스 측 호출 예시(범위 밖 참고용):

// GroupService
if (groupMemberRepository.existsByGroup_IdAndUser_IdNot(groupId, ownerUserId)) {
    throw new BizException(GroupErrorCode.OTHER_USER_EXIST_IN_GROUP);
}

추가로, 삭제된 그룹을 전역적으로 배제해야 한다면(soft-delete 유지), 파생 쿼리만으로는 Group.deletedAt 조건이 자동 적용되지 않습니다. 그런 요구가 있다면 커스텀 @query로 그룹 조인 후 조건을 명시하는 방식을 고려해주세요:

@Query("""
select (count(gm) > 0) 
from GroupMember gm 
join gm.group g 
where g.id = :groupId 
  and g.deletedAt is null 
  and gm.user.id <> :userId
""")
boolean existsActiveMemberOtherThanUser(@Param("groupId") Long groupId, @Param("userId") Long userId);

src/main/java/project/flipnote/group/controller/GroupController.java (1)

47-56: 다른 DELETE 엔드포인트와의 상태 코드 일관성 제안

동일 프로젝트 내 일부 DELETE는 200 OK를 반환합니다(예: GroupInvitationController). 가능하다면 모두 204로 통일해 클라이언트 처리를 단순화하는 것을 권장합니다.

참고(범위 밖 예시):

// GroupInvitationController
// return ResponseEntity.ok().build();
return ResponseEntity.noContent().build();

src/test/java/project/flipnote/group/service/GroupServiceTest.java (2)

241-269: 오너가 아닌 경우: 사용자 조회 스텁 ID 정렬 및 삭제 호출 방지 검증을 권장합니다

Line 260의 1L 하드코딩은 성공 케이스와 동일한 불안정성을 유발할 수 있습니다. 또한 음수 경로에서 delete 호출이 전혀 발생하지 않았음을 검증하면 회귀를 방지할 수 있습니다.

아래 변경을 제안합니다:

-		given(userProfileRepository.findByIdAndStatus(1L, UserStatus.ACTIVE)).willReturn(Optional.ofNullable(userProfile));
+		given(userProfileRepository.findByIdAndStatus(userProfile.getId(), UserStatus.ACTIVE)).willReturn(Optional.ofNullable(userProfile));
@@
 		BizException exception =
 			assertThrows(BizException.class, () -> groupService.deleteGroup(authPrinciple, 1L));
 
 		assertEquals(GroupErrorCode.USER_NOT_PERMISSION, exception.getErrorCode());
+		// 삭제 호출이 일어나지 않았는지 검증
+		then(groupRepository).should(never()).delete(any());

추가로 필요한 정적 임포트(테스트 파일 최상단, 기존 BDDMockito 스타일과 호환됩니다):

import static org.mockito.Mockito.*;

---

272-300: 그룹에 다른 유저가 존재하는 경우: 사용자 조회 스텁 ID 정렬 및 삭제 호출 방지 검증 추가

성공/다른 실패 케이스와 동일한 보완 사항입니다.

아래 변경을 제안합니다:

-		given(userProfileRepository.findByIdAndStatus(1L, UserStatus.ACTIVE)).willReturn(Optional.ofNullable(userProfile));
+		given(userProfileRepository.findByIdAndStatus(userProfile.getId(), UserStatus.ACTIVE)).willReturn(Optional.ofNullable(userProfile));
@@
 		BizException exception =
 			assertThrows(BizException.class, () -> groupService.deleteGroup(authPrinciple, 1L));
 
 		assertEquals(GroupErrorCode.OTHER_USER_EXIST_IN_GROUP, exception.getErrorCode());
+		// 삭제 호출이 일어나지 않았는지 검증
+		then(groupRepository).should(never()).delete(any());

필요 시 상단에 다음 임포트를 추가하세요:

import static org.mockito.Mockito.*;

src/main/java/project/flipnote/group/service/GroupService.java (2)

167-176: 불리언 로직 단순화 및 메서드 명시성 향상 권장

현재 메서드명(checkUserNotExistInGroup)과 반환 의미가 직관적이지 않고, 불리언 로직도 단순화 가능합니다. “오너만 존재하는지”를 나타내는 이름과 형태로 바꾸면 가독성이 좋아집니다.

아래처럼 단순화 및 명확화하고, 호출부도 함께 수정하는 것을 제안합니다.

-	private boolean checkUserNotExistInGroup(UserProfile user, Long groupId) {
-		long count = groupMemberRepository.countByGroup_idAndUser_idNot(groupId, user.getId());
-		if (count > 0) {
-			return false;
-		}
-		return true;
-	}
+	private boolean isOnlyMemberInGroup(Long groupId, Long userId) {
+		return groupMemberRepository.countByGroup_idAndUser_idNot(groupId, userId) == 0;
+	}

호출부 변경(아래 삭제 메서드 코멘트에 함께 제시):

---

195-219: 동시성 제어 부재: 멤버 추가/삭제와의 경쟁 상태 가능성

동일 PR 내 GroupMemberPolicyService.addGroupMember가 Redisson 분산락을 사용해 멤버 추가를 직렬화하는 반면, deleteGroup은 어떤 락도 사용하지 않습니다. 현재 로직은 “다른 멤버 없음”을 count 쿼리로 확인한 뒤 삭제합니다. 멤버 추가 작업과 경합 시 다음 이슈가 발생할 수 있습니다:

• 삭제 시점 count=0 → 직후 멤버 추가(잠금으로 보호) → 삭제 커밋 순으로 실행될 경우, 정책상 허용되지 않는 타이밍 이슈가 생길 수 있음.

최소한 동일한 락 키(예: "group_member_lock:{groupId}")로 삭제도 보호하거나, DB 레벨에서 그룹 행에 대한 SELECT ... FOR UPDATE 기반 조회로 일관성을 확보하는 것을 권장합니다.

락 적용 예시(의존성 주입 및 예외 처리 생략):

// 1) Redisson 락 적용
String lockKey = "group_member_lock:" + groupId;
RLock lock = redissonClient.getLock(lockKey);
boolean isLocked = lock.tryLock(2, 3, TimeUnit.SECONDS);
try {
    if (!isLocked) throw new BizException(CommonErrorCode.SERVICE_TEMPORARILY_UNAVAILABLE);

    Group lockedGroup = groupRepository.findByIdForUpdate(groupId)
        .orElseThrow(() -> new BizException(GroupErrorCode.GROUP_NOT_FOUND));

    // 이후 validateUser, validateGroupInUser, 오너 검증, isOnlyMemberInGroup(groupId, user.getId()) 재검증
    // 마지막에 delete
} finally {
    if (isLocked && lock.isHeldByCurrentThread()) lock.unlock();
}

또한, Enum 비교는 동등성(equals) 대신 동치(==) 비교가 관용적입니다. 작은 개선을 제안합니다.

-		if (!groupMember.getRole().equals(GroupMemberRole.OWNER)) {
+		if (groupMember.getRole() != GroupMemberRole.OWNER) {
 			throw new BizException(GroupErrorCode.USER_NOT_PERMISSION);
 		}

위의 메서드 명/호출부 개선을 함께 반영하려면:

-		if (!checkUserNotExistInGroup(user, groupId)) {
+		if (!isOnlyMemberInGroup(groupId, user.getId())) {
 			throw new BizException(GroupErrorCode.OTHER_USER_EXIST_IN_GROUP);
 		}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/main/java/project/flipnote/group/exception/GroupErrorCode.java (3)

16-16: 한글 문구 띄어쓰기/조사 보정

문구의 자연스러움과 가독성을 위해 다음과 같이 수정 제안합니다.

-	OTHER_USER_EXIST_IN_GROUP(HttpStatus.CONFLICT, "GROUP_007", "그룹내 오너 제외 유저가 존재합니다."),
+	OTHER_USER_EXIST_IN_GROUP(HttpStatus.CONFLICT, "GROUP_007", "그룹 내 오너를 제외한 유저가 존재합니다."),

---

15-15: 용어 일관성(유저/회원/멤버) 정리 제안

동일 enum 내에서 ‘유저’(Line 15)와 ‘회원’(Line 18)이 혼용됩니다. 코드 전반에서는 ‘멤버/사용자’도 함께 쓰이고 있어 혼재가 커 보입니다. 이번 PR에서 강제 변경까지는 아니지만, 이 파일만큼은 통일하는 것을 권장합니다. 아래 예시는 ‘멤버’ 기준입니다.

-	USER_NOT_IN_GROUP(HttpStatus.NOT_FOUND, "GROUP_003", "그룹에 유저가 존재하지 않습니다."),
+	USER_NOT_IN_GROUP(HttpStatus.NOT_FOUND, "GROUP_003", "해당 사용자는 그룹의 멤버가 아닙니다."),
...
-	ALREADY_GROUP_MEMBER(HttpStatus.CONFLICT, "GROUP_005", "이미 그룹 회원입니다.");
+	ALREADY_GROUP_MEMBER(HttpStatus.CONFLICT, "GROUP_005", "이미 그룹 멤버입니다.");

팀에서 합의된 용어집(사용자/유저/멤버/회원 중 택1)이 있다면 그 기준에 맞춰 일괄 정리하는 이슈로 분리하는 것도 좋습니다.

Also applies to: 18-18

---

13-13: 하드코딩된 최대 인원(100) → 상수/설정 연동으로 드리프트 방지

비즈니스 정책 값(100)이 메시지에 하드코딩되어 있어, 정책 변경 시 메시지와 실제 검증 로직이 어긋날 수 있습니다. 공용 상수나 설정값을 참조하도록 변경을 권장합니다.

• 예시: 정책 상수 도입

// project/flipnote/group/GroupPolicy.java
public final class GroupPolicy {
    private GroupPolicy() {}
    public static final int MAX_MEMBERS = 100;
}

• 해당 enum 항목 수정

-	INVALID_MAX_MEMBER(HttpStatus.BAD_REQUEST, "GROUP_001", "최대 인원 수는 1 이상 100 이하여야 합니다."),
+	INVALID_MAX_MEMBER(HttpStatus.BAD_REQUEST, "GROUP_001",
+		"최대 인원 수는 1 이상 " + GroupPolicy.MAX_MEMBERS + " 이하여야 합니다."),

또는 메시지를 일반화하여 숫자를 배제하는 방법도 있습니다(“최대 인원 수가 허용 범위를 초과했습니다.”).

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.